-
Notifications
You must be signed in to change notification settings - Fork 4.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
New service: DeviceRegistry
+ new resources: azurerm_device_registry_asset
and azurerm_device_registry_asset_endpoint_profile
#28399
base: main
Are you sure you want to change the base?
Conversation
DeviceRegistry
DeviceRegistry
…d only fields per terraform requirement
…azurerm into rylo/adr-terraform
…to local cluster only
DeviceRegistry
DeviceRegistry
DeviceRegistry
DeviceRegistry
+ new resources: azurerm_device_registry_asset
and azurerm_device_registry_asset_endpoint_profile
r := AssetTestResource{} | ||
|
||
if os.Getenv(ASSET_CUSTOM_LOCATION_NAME) == "" || os.Getenv(ASSET_RESOURCE_GROUP_NAME) == "" { | ||
t.Skipf("Skipping test due to missing environment variables %s and/or %s", ASSET_CUSTOM_LOCATION_NAME, ASSET_RESOURCE_GROUP_NAME) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For both asset and asset endpoint profile resource tests, we require env vars ARM_DEVICE_REGISTRY_CUSTOM_LOCATION
and ARM_DEVICE_REGISTRY_RESOURCE_GROUP
to be set or else the tests will fail (to prevent this we skip the tests).
however, the resource group, custom location, and the AIO cluster must be pre-existing this test run. Investigation and efforts to get the acceptance tests to create and provision an AIO cluster are flakey, take a long time to run (45+ minutes), and we found that the test steps delete the cluster resources before the asset/asset endpoint profile resource tests run (causing the latter ones to fail). awaiting response from Hashicorp and Terraform PMs what the best course of action is here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @mryanlo,
In short it would be acceptable for the tests to run against existing infrastructure for the time being to help move this PR forwards, but the maintainers do need to be able to independently run the acceptance tests for new RPs and resources that are in the provider. I'd like to better understand the procedure required to test these resources so that we can setup the tests to run without a dependence on pre-existing resources.
We have acceptance tests in the provider for arc enabled clusters where we need to provision the prerequisite network resources and a VM that runs the necessary setup scripts for the cluster.
Would you be able to take a look and let me know if there are parts of this setup that you would be able to re-use and what modifications you would need to make for setting up AIO on the cluster specifically? e.g. perhaps the setup script etc.
Once we have a better understanding of what's required we can begin looking at ways to cut down the provisioning time and improving the stability of the test.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @stephybun thanks for reaching out! the exact steps to install AIO onto a cluster are found here: https://review.learn.microsoft.com/en-us/azure/iot-operations/get-started-end-to-end-sample/quickstart-deploy?branch=main
I've seen that link you sent me in the past and have been unable to successfully modify it for AIO's purposes. This is due to the complexity and flakiness of the bash/python script that the VM must run, and our restrictions with the linux distro for installing AIO (you need linux Ubuntu 24.04 with a k3s cluster, not kind cluster to succeed). Additionally there are a bunch of az cli commands that must run and this process is the only available way to install AIO on a cluster, so we need to install az cli and k3s.
If you'd like to see my latest attempt at modifying the scripts to get these to work (the tests will try to sequentially first create a single cluster with AIO installed and then run the acc tests to create resources on only that cluster), here are links:
https://github.com/mryanlo/terraform-provider-azurerm/blob/rylo/adr-tf-tests/internal/services/deviceregistry/device_registry_assetendpointprofile_resource_test.go
https://github.com/mryanlo/terraform-provider-azurerm/tree/rylo/adr-tf-tests/internal/services/deviceregistry/testdata
Thanks, and let me know if you have any more questions about this process.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: my code modifies the acceptance tests to run via a managed identity since due to security reasons we are not allowed to make service principals to run terraform anymore.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@stephybun is there a way to ignore the terraform plan check in acceptance tests from running on the VM portions? we don't care about the VM changing since it's not the resource that we want to test. however, one of the issues with my script above is that even if the AIO installation succeeds, the test flags the terraform plan step of the VM as modified after creating.
param.Properties.DefaultTopic.Path = pointer.To(config.DefaultTopicPath) | ||
} | ||
if defaultTopicRetainChanged { | ||
// Bug with `go-azure-sdk` library: you can't set retain to null because empty string will cause |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Linking issue hashicorp/go-azure-sdk#1143 and PR hashicorp/pandora#4633
The go sdk does not support sending null properties. After the PR goes in, this issue can be fixed but might require a code change here
This PR introduces support for the Device Registry service, comprising of:
Resources
Community Note
Description
Swagger: https://github.com/Azure/azure-rest-api-specs/tree/main/specification/deviceregistry/resource-manager/Microsoft.DeviceRegistry/stable/2024-11-01
Azure Doc: https://learn.microsoft.com/en-us/azure/iot-operations/discover-manage-assets/howto-manage-assets-remotely?tabs=cli
PR Checklist
For example: “
resource_name_here
- description of change e.g. adding propertynew_property_name_here
”Changes to existing Resource / Data Source
Testing
The acceptance tests for
azurerm_device_registry_asset
andazurerm_device_registry_asset_endpoint_profile
require an arc-enabled kubernetes cluster with Azure IoT Operations' service extension installed on the cluster in order to pass. Creating this cluster in the terraform acceptance tests has been unsuccessful, takes over 45 minutes to complete, and the installation of AIO script has proven to be flakey. Once the cluster is setup, you need to add these environment variables to pass:Local tests against a pre-existing AIO cluster:
Change Log
Below please provide what should go into the changelog (if anything) conforming to the Changelog Format documented here.
DeviceRegistry
- support for thedevice_registry
service [GH-00000]azurerm_device_registry_asset
[GH-00000]azurerm_device_registry_asset_endpoint_profile
[GH-00000]This is a (please select all that apply):
Related Issue(s)
Fixes #0000
Note
If this PR changes meaningfully during the course of review please update the title and description as required.